Add seal-day backend action for daily snapshots#1037
Conversation
Introduces DailySnapshot entity for the seal-day action (#1017). Includes validation (no future dates, non-empty userId), idempotent Seal() method, and computed IsSealed property.
Application layer for seal-day (#1017): repository interface with user+date lookup and sealed-days range query, service interface with seal and status check, and service implementation with get-or-create idempotent seal logic.
Adds DailySnapshotRepository, EF configuration with unique index on (UserId, Date), DbContext registration, UnitOfWork integration, DI wiring, and TodayController with POST/GET /api/today/seal endpoints. Closes the full vertical slice for #1017.
Creates DailySnapshots table with unique index on (UserId, Date) for the seal-day feature (#1017).
Updates all 9 fake/stub IUnitOfWork implementations in API tests to include the new IDailySnapshotRepository DailySnapshots member.
Domain tests: construction validation, seal idempotency, future date rejection, IsSealed property. Service tests: seal new day, seal already-sealed day (idempotent), empty userId validation, future date rejection, get-seal-status for missing/sealed/unsealed snapshots.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Adversarial Self-ReviewConcurrency: two simultaneous seal requestsThe unique index
Verdict: Safe. The worst case is a retriable 409 conflict, never data corruption. Date timezone edge casesThe
Verdict: Correct for a UTC-normalized system. If the product later needs timezone-aware day boundaries, the API contract ( Migration correctness
Verdict: Correct. Idempotency guarantees
Verdict: Strong idempotency at all layers. No issues found that require fixing. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Daily Seal' feature, which allows users to mark a specific date as sealed. The changes include the new DailySnapshot domain entity, a corresponding repository, and the DailySealService to handle sealing logic and status checks. A new TodayController provides the API endpoints for these operations. Feedback focuses on enhancing the robustness of the implementation by handling unique constraint violations for daily snapshots within the UnitOfWork to prevent unhandled exceptions during concurrent requests. Additionally, it is recommended to propagate CancellationToken throughout the asynchronous call stack, from the controller actions down to the repository layer, to ensure proper request cancellation support.
Adversarial Review -- Findings & FixesReviewed all 24 files in this PR. Addressing the 6 Gemini bot comments plus additional verification. Issues Found & FixedHIGH: Missing DailySnapshot unique constraint handler in UnitOfWork.cs
MEDIUM: Missing CancellationToken propagation (4 locations)
Additional Checks (All OK)
Build & Test Results
|
…nToken through seal flow Fix concurrent seal requests causing unhandled 500 errors by adding DailySnapshot to UnitOfWork.TryResolveRecoverableUniqueConflicts, matching the existing Notification/UserPreference pattern. Add CancellationToken to IDailySealService, DailySealService, and TodayController so the ASP.NET pipeline can cancel in-flight seal requests. Update test mock setups to use It.IsAny<CancellationToken>().
Fresh Adversarial Review (Round 3)CRITICAL1. Concurrent seal race returns fabricated response data (Confidence: 95)
The losing request reports Fix: After IMPORTANT2. No controller-level integration tests (Confidence: 85) — Domain and service tests exist (28 tests), but no HTTP pipeline test verifying routing, claims extraction, or status code mapping for Verified Non-Issues
|
After SaveChangesAsync, re-fetch the snapshot from the database to detect when TryResolveDuplicateDailySnapshotConflicts detached our in-memory entity. If the persisted snapshot has a different Id, the other request won the race — return WasAlreadySealed: true with the winner's SealedAt timestamp instead of stale in-memory values.
Resolve add/add conflict in TodayController.cs by combining both the cadence endpoint (from main via PR #1031) and the seal-day endpoints (from this branch). Both ICadenceService and IDailySealService are now injected via constructor DI.
# Conflicts: # backend/src/Taskdeck.Api/Controllers/TodayController.cs # frontend/taskdeck-web/tests/e2e/smoke.spec.ts
1 participant
Summary
Closes #1017. Implements the backend 'seal day' snapshot action required by PAPER-08.
DailySnapshotentity withSeal(DateTimeOffset)(idempotent -- second invoke is a no-op),IsSealedcomputed property, validation (no future dates, non-empty userId)IDailySnapshotRepositoryinterface (user+date lookup, sealed-days range query),IDailySealService/DailySealServicewith get-or-create + idempotent seal logic, response DTOsDailySnapshotRepository, EF Core configuration with unique index on(UserId, Date), EF migration forDailySnapshotstable, DI wiring inUnitOfWorkandDependencyInjectionTodayControllerwithPOST /api/today/seal(seal a day) andGET /api/today/seal?date=...(check seal status), claims-first identity viaAuthenticatedControllerBaseAdversarial self-review findings
Concurrency (two simultaneous seal requests): The unique index on
(UserId, Date)provides database-level protection against duplicate snapshots. If two requests race to create a snapshot for the same user+date, one will get aDbUpdateExceptioncaught byUnitOfWork.SaveChangesAsyncwhich wraps it in aDomainExceptionwithConflicterror code. TheSeal()method itself is idempotent, so if both requests find an existing row, both succeed harmlessly.Date timezone handling: The service uses
DateTimeOffset.UtcNowconsistently for thenowparameter. TheDateOnlycomparison in the domain constructor usesDateOnly.FromDateTime(now.UtcDateTime)to ensure UTC-based date boundary. Thedateparameter isDateOnly(no timezone ambiguity). The API acceptsDateOnlyfrom the request body/query, which serializes asYYYY-MM-DDwith no timezone component -- the caller decides which calendar day to seal.Migration correctness: The migration creates the table with the correct columns, primary key, and unique composite index on
(UserId, Date). TheDown()method drops the table cleanly.Idempotency guarantee: Enforced at two levels -- domain (
Seal()is a no-op if already sealed) and service (returnsWasAlreadySealed: truewithout mutation). TheUpdatedAtconcurrency token prevents lost updates if the row changes between read and write.Missing edge case: No explicit transaction wrapping in
SealDayAsyncfor the create-then-seal path. With EF Core change tracking, both theAddAsyncand the implicitSeal()mutation are persisted in a singleSaveChangesAsynccall, which is atomic. This is safe.Test plan
dotnet build backend/Taskdeck.sln -c Releasepasses (0 errors)